-
Notifications
You must be signed in to change notification settings - Fork 125
Fix various warnings and cleanup samples #579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* Applied every IDE suggestion to the samples. We should keep the sample code copy-pasteable into the IDE without warnings. * Gradle mentions that compileOnly dependencies don't work in Wasm, so changed the dependency of `kotlinx-datetime-zoneinfo` from compileOnly to api. * Replaced the non-exhaustive-when error suppressions with a runtime check to be able to diagnose the issue better if we indeed make an error at some point. The runtime check can be removed if the compiler becomes smarter. * Fixed various other warnings both in test/ and src/.
| commonMain { | ||
| dependencies { | ||
| compileOnly(project(":kotlinx-datetime")) | ||
| api(project(":kotlinx-datetime")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We intended compileOnly here in order not to have kotlinx-datetime in dependencies of the published artifact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true, but that doesn't seem to work.
If a project depends on kotlinx-datetime-zoneinfo, but doesn't depend on kotlinx-datetime, you get:
> Task :kotlinx-datetime-test-with-wasi-timezones-without-core:compileTestDevelopmentExecutableKotlinWasmWasi FAILED
w: KLIB resolver: Could not find "org.jetbrains.kotlinx:kotlinx-datetime" in [/home/dkhalansky/.local/share/kotlin/daemon]
w: KLIB resolver: Could not find "org.jetbrains.kotlinx:kotlinx-serialization-core" in [/home/dkhalansky/.local/share/kotlin/daemon]
e: <org.jetbrains.kotlinx:kotlinx-datetime-zoneinfo> @ /home/dkhalansky/IdeaProjects/kotlinx-datetime/timezones/full/build/convertedTimesZones-full/src/internal/tzData/tzData.kt:619:36: Constructor 'IllegalTimeZoneException.<init>' can not be called: No constructor found for symbol 'kotlinx.datetime/IllegalTimeZoneException.<init>|<init>(kotlin.String){}[0]'
e: <org.jetbrains.kotlinx:kotlinx-datetime-zoneinfo> @ /home/dkhalansky/IdeaProjects/kotlinx-datetime/timezones/full/common/src/TimeZonesInitializer.kt:13:9: Anonymous object uses unlinked class symbol 'kotlinx.datetime.internal/TimeZonesProvider|null[0]'
e: <org.jetbrains.kotlinx:kotlinx-datetime-zoneinfo> @ /home/dkhalansky/IdeaProjects/kotlinx-datetime/timezones/full/common/src/TimeZonesInitializer.kt:13:9: Constructor '<init>' can not be called: Anonymous object uses unlinked class symbol 'kotlinx.datetime.internal/TimeZonesProvider|null[0]'
e: <org.jetbrains.kotlinx:kotlinx-datetime-zoneinfo> @ /home/dkhalansky/IdeaProjects/kotlinx-datetime/timezones/full/common/src/TimeZonesInitializer.kt:13:9: Anonymous object literal can not be evaluated: Expression uses unlinked class symbol 'kotlinx.datetime.internal/TimeZonesProvider|null[0]'
e: <org.jetbrains.kotlinx:kotlinx-datetime-zoneinfo> @ /home/dkhalansky/IdeaProjects/kotlinx-datetime/timezones/full/common/src/TimeZonesInitializer.kt:12:31: Function 'initializeTimeZonesProvider' can not be called: No function found for symbol 'kotlinx.datetime.internal/initializeTimeZonesProvider|initializeTimeZonesProvider(kotlinx.datetime.internal.TimeZonesProvider){}[0]'
e: <missing declarations>: No class found for symbol 'kotlinx.datetime.internal/TimeZonesProvider|null[0]'
e: <missing declarations>: No class found for symbol 'kotlinx.datetime/IllegalTimeZoneException|null[0]'
e: <missing declarations>: No constructor found for symbol 'kotlinx.datetime/IllegalTimeZoneException.<init>|<init>(kotlin.String){}[0]'
e: <missing declarations>: No function found for symbol 'kotlinx.datetime.internal/initializeTimeZonesProvider|initializeTimeZonesProvider(kotlinx.datetime.internal.TimeZonesProvider){}[0]'
e: <missing declarations>: No function found for symbol 'kotlinx.datetime.internal/TimeZonesProvider.zoneDataByName|zoneDataByName(kotlin.String){}[0]'
e: <missing declarations>: No function found for symbol 'kotlinx.datetime.internal/TimeZonesProvider.getTimeZones|getTimeZones(){}[0]'
e: There are linkage errors reported by the partial linkage engineAdditionally, there's a warning:
w: ⚠️ Unsupported `compileOnly` Dependencies in Kotlin Targets
A compileOnly dependency is used in targets: Kotlin/Wasm.
Dependencies:
- org.jetbrains.kotlinx:kotlinx-datetime:0.7.1-SNAPSHOT (source sets: wasmWasiMain)
Using compileOnly dependencies in these targets is not currently supported, because compileOnly dependencies must be present during the compilation of projects that depend on this project.
To ensure consistent compilation behaviour, compileOnly dependencies should be exposed as api dependencies.
Example:
kotlin {
sourceSets {
nativeMain {
dependencies {
compileOnly("org.example:lib:1.2.3")
// additionally add the compileOnly dependency as an api dependency:
api("org.example:lib:1.2.3")
}
}
}
}
This warning can be suppressed in gradle.properties:
kotlin.suppressGradlePluginWarnings=IncorrectCompileOnlyDependencyWarning
Please expose compileOnly dependencies as api dependencies.
One upside of the current approach that I can think of is that people can't accidentally depend on kotlinx-datetime-zoneinfo and call it a day, they have to specify the kotlinx-datetime dependency anyway, but the error message is not all that clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One upside of the current approach that I can think of is that people can't accidentally depend on kotlinx-datetime-zoneinfo and call it a day, they have to specify the kotlinx-datetime dependency anyway
Yes, the intent was something like that + not to introduce additional version requirements on kotlinx-datetime with dependency on kotlinx-datetime-zoneinfo.
Probably we can use api dependency for now, while versions of kotlinx-datetime and -zoneinfo are in sync and then think about it one more time when we switch to a more stable and flexible versioning.
0a16b7f to
cf57e95
Compare
kotlinx-datetime-zoneinfofrom compileOnly to api.